-
Notifications
You must be signed in to change notification settings - Fork 536
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enforce compute cluster version #1601
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this check potentially too restrictive? do we actually need an exact match? or we just don't know?
@@ -451,6 +451,7 @@ def convert_dataset_hf_from_args( | |||
ValueError: If the output directory already contains the requested splits | |||
ValueError: If `concat_tokens` is set but `tokenizer` is not | |||
""" | |||
os.environ['WORLD_SIZE'] = '1' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are these changes supposed to be included?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually going to close this pr, during testing seems like every version I tested worked (strangely), so perhaps old bug was actually transient, and was solved not by the version change but simply restarting the cluster. Will revisit if it comes up again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original motivation is we don't know what works and what gets deprecated
Not needed as all versions currently work... see above comment. |
To resolve:
https://databricks.slack.com/archives/C06RXC1NPQC/p1727371779924089
Essentially, we check if the compute runtime version is higher than our
databricks-connect
version to determine whether to use 'dbconnect' or not, but we don't check if the compute runtime version is too high. There is no formula to determine which version is too high, so we should just enforce the customer using a computer that has the same major runtime as thedatabricks-connect
versionTest:
Error should be thrown:
with the following compute:
But should work: